-
Notifications
You must be signed in to change notification settings - Fork 107
feat(AggLayer): UPDATE_GER note
#2333
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
We should also add a safety check similar to #2173 to ensure that only the bridge account can consume this note. I added a comment to the mentioned issue so that tackling it could address both |
partylikeits1983
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
For now we just store the 256 bit GER in 2 slots, but not in an array just yet? Assuming will update this once the double word array PR lands: #2299
Exactly - but since we didn't store the GER at all until now, this should allow some minimal degree of testing against a real GER |
* feat: UPDATE_GER note outline * feat: working update ger note * chore: swap upper, lower GER parts * lint: regen error file
| #! This note can only be consumed by the specific agglayer bridge account whose ID is provided | ||
| #! in the note attachment (target_account_id). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not currently enforced, right? If so, might be good to add a comment about this.
| const GER_UPPER_STORAGE_SLOT=word("miden::agglayer::bridge::ger_upper") | ||
| const GER_LOWER_STORAGE_SLOT=word("miden::agglayer::bridge::ger_lower") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of comments on these:
- Do we actually need to store the full
GER? it seems like, the claim note inputs can be used to compute theGERand then, we just need to check if theGERcomputed from the note matches theGERin the bride. For this, we just need to storeGERcommitment (i.e., RPO hash). This would mean we need only one storage slot. - Maybe we should make the storage slot into a map:
hash(ger) |-> 1/0. This way, given aGER, we can compute its hash to reduce it to a single word, and then do a lookup to check if theGERis known or not. - It feels a bit odd that
GERupdate functionality is in thebridge_inmodule. It probably would make sense to have a separatebridge_operatormodule for things like this.
UPDATE_GERnote script that carries two storage words[GER_LOWER[4], GER_UPPER[4]]update_gerprocedure on the bridgecreate_bridge*helper functionscreate_bridge_account_buildernow uses bothBridgeIn&BridgeOutcomponents in a single accountBridgeOutdoesn't need themiden::agglayer::bridgestorage slot it used to haveBridgeIngets two storage slots to store single values (upper & lower limbs of the GER)closes #2159
closes #2137